Skip to content

Fix CI workflows: remove obsolete Node.js setup step after Playwright TypeScript → C# migration#18992

Merged
Skrypt merged 2 commits intoskrypt/playwrightfrom
copilot/sub-pr-18987
Mar 12, 2026
Merged

Fix CI workflows: remove obsolete Node.js setup step after Playwright TypeScript → C# migration#18992
Skrypt merged 2 commits intoskrypt/playwrightfrom
copilot/sub-pr-18987

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 12, 2026

CI was failing across all workflows because node-version: "15" (EOL, unavailable on GitHub Actions runners) was left in place after migrating Playwright functional tests from TypeScript to C#. Node.js is no longer needed — the functional tests now run via dotnet test with Microsoft.Playwright.

Changes

  • Removed the setup-node step from pr_ci.yml, main_ci.yml, preview_ci.yml, and release_ci.yml

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh gh run list --branch copilot/sub-pr-18987 --limit 10 (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Refactor all Cypress functional tests to Playwright Fix CI workflows: remove obsolete Node.js setup step after Playwright TypeScript → C# migration Mar 12, 2026
Copilot AI requested a review from Skrypt March 12, 2026 20:22
@Skrypt Skrypt marked this pull request as ready for review March 12, 2026 20:23
@Skrypt Skrypt merged commit 6b25b32 into skrypt/playwright Mar 12, 2026
3 checks passed
@Skrypt Skrypt deleted the copilot/sub-pr-18987 branch March 12, 2026 20:23
Skrypt added a commit that referenced this pull request Mar 28, 2026
* Playwright migration from Cypress

Refactor all Cypress functional tests in a 1:1 migration to Playwright.

* Enable corepack in CI workflows for Yarn 4 compatibility

The CI runners have Yarn 1.x globally but the project uses Yarn 4.9.4
via packageManager field. Adding corepack enable ensures the correct
Yarn version is used. Also removes unnecessary cd into subdirectory
for functional_all_db.yml since yarn workspace commands run from root.

* remove failing doc

* Do not dotnet build twice

* cache playwright browsers and fix tests

* fix tests

* fix playwright cache usage on CI

* Cleanup / docs / .slnx / .csproj

* fix csproj

* Refactor Playwright functional tests from TypeScript to C# (#18991)

* Initial plan

* Refactor Playwright functional tests from TypeScript to C#

- Remove TypeScript Playwright test project (test/OrchardCore.Tests.Playwright)
- Create C# Playwright test project at test/OrchardCore.Tests.Functional
- Add xUnit v3 test classes mirroring all original TS tests
- Add helper classes for auth, tenants, configuration, lifecycle, etc.
- Update all CI workflows to use dotnet test instead of yarn/npm
- Update OrchardCore.slnx, Directory.Packages.props, AGENTS.md
- Revert root package.json (remove Playwright workspace/scripts)

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Fix code review feedback: use base-32 encoding and static members

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* update yarn.lock

* Fix CI workflows: remove obsolete Node.js setup step after Playwright TypeScript → C# migration (#18992)

* Initial plan

* Fix CI workflows by removing obsolete Node.js setup step

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Fix missing --project on command

* Add Playwright cache CI step

* Update test/OrchardCore.Tests.Functional/OrchardCore.Tests.Functional.csproj

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update test/OrchardCore.Tests.Functional/Helpers/TenantHelper.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Only delete migrations recipe file if it was created by the test run (#18994)

* Initial plan

* Fix: only delete migrations recipe if it was copied by us in this run

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Refactor CMS functional tests to use CmsTestBase (#18995)

* Initial plan

* Refactor CMS tests to use CmsTestBase, eliminating duplicate tenant setup logic

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Update test/OrchardCore.Tests.Functional/Helpers/TenantHelper.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix BrowserContext leak in OrchardTestFixture.CreatePageAsync (#18996)

* Initial plan

* Fix BrowserContext leak in CreatePageAsync by closing context when page closes

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Fix CS1513 build error in TenantHelper.cs from malformed review suggestion merge (#18997)

* Initial plan

* Fix CS1513 syntax error in TenantHelper.cs - malformed review suggestion merge

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Playwright migration from Cypress (#18998)

* Initial plan

* fix: address review comments - HttpClient timeout and external site setup detection

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Playwright migration from Cypress (#19012)

* Initial plan

* Fix race condition in parallel CMS/MVC tests and refactor helpers from skrypt/vue-3

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Refactor functional tests to use WebApplicationFactory and enable parallel execution

Replace process-based app hosting (Process.Start) with ASP.NET Core's
WebApplicationFactory, running Kestrel on dynamic ports in-process. This
enables same-process debugging and removes the need for separate build steps.

- Convert all helper classes to IPage extension methods for fluent syntax
- Add Playwright tracing support (enabled via PLAYWRIGHT_TRACING env var)
  with trace artifacts uploaded on CI failure
- Add in-memory logger provider that asserts no Error/Critical log entries
  in test teardown
- Remove tenant-per-test pattern: non-SaaS tests now set up the site
  directly with their recipe instead of creating a SaaS tenant intermediary
- Each test class gets its own IClassFixture with isolated app instance
  and unique App_Data directory, enabling xUnit parallel execution
- Remove CmsTestCollection/MvcTestCollection serial collection constraints

* Fix recipe file race condition when fixtures initialize in parallel

Add a static lock around CopyRecipe and DeleteRecipe to prevent
concurrent fixtures from racing on the same migrations.recipe.json file.

* Replace WebApplicationFactory with direct host builder and remove deprecated Razor runtime compilation

WebApplicationFactory's internal TestServer cast fails when using Kestrel
for Playwright. Replace with OrchardTestServer that builds and starts the
host directly using WebApplication.CreateBuilder with the correct
ApplicationName for module discovery.

- Remove Razor runtime compilation (deprecated in .NET 10) and the
  Development+refs guard that skipped pre-compiled views, so module views
  are always served from compiled assemblies regardless of environment
- Change SharedViewCompilerProvider._compiler from static to instance to
  fix view cache corruption when running multiple hosts in-process
- Use PostConfigure<ShellOptions> to set per-instance app data paths,
  avoiding env var race conditions under parallel execution
- Add log assertions to MVC tests, split SaasFixture to its own file
- Add src/OrchardCore.Cms.Web/Recipes/ to .gitignore

* Clean up App_Data_Tests directories after functional tests complete

Stop the server with StopAsync before disposal so file handles are
released, clear the SQLite connection pool to unlock database files,
then delete the per-fixture App_Data_Tests directories.

* Set unique table prefix per fixture to isolate shared databases in CI

When running against MySQL, PostgreSQL, or SQL Server, all parallel
fixtures share a single database. Each fixture now sets a unique table
prefix during site setup so their tables don't collide. The prefix field
is only filled when visible (non-SQLite providers).

* Fix table prefix

* Set table prefix via configuration instead of setup form UI

The #TablePrefix form field is hidden when the database provider is
pre-configured, so setting it via JS or FillAsync silently fails. Pass
the table prefix through OrchardCore:TablePrefix in configuration, which
the setup controller reads from shell settings and applies automatically
when DatabaseConfigurationPreset is true.

* Create unique database per fixture for parallel CI execution

Instead of sharing a single database with table prefixes (which still
causes race conditions during setup), each fixture now appends its name
to the database name in the connection string (e.g., app_AgencyFixture).
The database is created on the fly via a connection to the server's
default database (postgres, mysql, or master). Parallelism is limited
to 2 threads via xunit.runner.json to avoid overwhelming CI runners.

* Fix database isolation for shared database CI jobs

ShellSettingsManager re-adds env var providers with highest priority,
so builder.Configuration overrides for the connection string were
silently ignored. Set OrchardCore__ConnectionString as an actual env var
instead, serialized with a semaphore since env vars are process-wide.

Also fix PostgreSQL CREATE DATABASE syntax (IF NOT EXISTS is not valid
in PostgreSQL) by checking pg_database first.

* Fix cascading database name suffix when fixtures modify the env var

Each fixture read OrchardCore__ConnectionString from the env var after
the previous fixture had already appended its suffix, producing names
like test_AgencyFixture_BlogFixture. Capture the original connection
string once at static initialization and always compute from that.

* Simplified OrchardTestServer

removed the database creation/manipulation code and semaphore. Serial execution with maxParallelThreads: 1 eliminates all shared database conflicts.

* different db names

* Fix CI workflow: update actions/cache to Node.js 24, add tracing and fix artifact paths (#19029)

* Initial plan

* Fix CI workflow: update actions/cache to v5.0.4, add tracing and correct artifact paths

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* add doc

* Replace env var usage

* trying to fix it

* fix concurrency issues

* fix concurrency issues

* force production mode

* remove env var for testing

* add missing file

* Intentionally failing functional test

* Revert "Intentionally failing functional test"

This reverts commit 0ebd093.

* Removing unnecessary namespace imports

* Re-adding actually needed usings

* Replace custom InMemoryLoggerProvider with FakeLoggerProvider in functional tests (#19041)

* Initial plan

* Replace InMemoryLoggerProvider with FakeLoggerProvider from Microsoft.Extensions.Diagnostics.Testing

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>
Agent-Logs-Url: https://github.com/OrchardCMS/OrchardCore/sessions/93198a10-d525-49fa-b98f-a1b2aac3ce7d

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Playwright migration from Cypress (#19042)

* Initial plan

* Address Piedone review feedback: Lock class, trace naming, warnings in assertions

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>
Agent-Logs-Url: https://github.com/OrchardCMS/OrchardCore/sessions/7cececba-1b43-4216-92d7-45348f9f7470

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

* Playwright - Remove deprecated Razor runtime compilation and enable Hot Reload for module views (#19033)

* Remove deprecated Razor runtime compilation and enable Hot Reload for module views

Remove AddRazorRuntimeCompilation() and all supporting code deprecated in
.NET 10: SharedViewCompilerProvider, RazorCompilationOptionsSetup,
RazorCompilationFileProviderAccessor, DevelopmentViewsFeature, and the
refs folder checks in ShellViewFeatureProvider and AdminPageRouteModelProvider.

Replace RazorCompilationFileProviderAccessor with ViewFileProviderAccessor
that composes file providers directly without the deprecated
MvcRazorRuntimeCompilationOptions.

Register ModuleProjectRazorFileProvider and ApplicationViewFileProvider on
the host's ContentRootFileProvider in Development mode so Hot Reload can
detect and apply .cshtml changes from module source directories.

Update VS Code tasks.json and launch.json for dotnet watch with Hot Reload
support and target framework selection via input prompt.

* Test parallel execution

* Test full parallelism

* Add longer default timeout

* keep serial mode

Because we can't predict how long it will take for DB setups

* Remove unnecessary SetDefaultTimeout from OrchardTestFixture (#19050)

* Initial plan

* Remove unnecessary SetDefaultTimeout from OrchardTestFixture

Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>
Agent-Logs-Url: https://github.com/OrchardCMS/OrchardCore/sessions/e77a91b3-5954-41c3-aefd-52497145a774

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Skrypt <3228637+Skrypt@users.noreply.github.com>

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>

* Fix All databases functional test

* Revert Razor compilation removal (moved to skrypt/playwright-comp)

Keep only the static->instance fix for SharedViewCompilerProvider to
prevent view cache corruption when multiple test hosts run in-process.
All Razor runtime compilation and Hot Reload changes are moved to the
skrypt/playwright-comp branch.

* Clean up CI workflows and harden functional test helpers

- Remove dead ORCHARD_APP env var from all CI workflows and AGENTS.md
  (MVC vs CMS selection is handled entirely by --filter-class)
- Align artifact upload paths in release_ci and preview_ci with
  main_ci/pr_ci: use App_Data_Tests_* wildcard for per-fixture dirs,
  include MVC logs and Playwright traces
- Fix App_Data_Tests cleanup glob in release_ci and preview_ci to
  match per-fixture directory naming
- Use parameterized queries for database existence checks in
  OrchardTestServer and properly escape provider-specific delimiters
  in CREATE DATABASE DDL; add allowlist validation for database names
- Track whether CopyRecipe() created the file and only delete on
  dispose when the fixture owns it, preventing accidental deletion of
  developer-maintained recipes
- Update functional tests README to reflect current code: FakeLoggerProvider,
  fixture hierarchy, accurate helper/test listings, and corrected env vars

* Test parallel execution on Github

* Enable parallel test execution and eliminate shared mutable state

Replace AppLifecycleHelper recipe file copy/delete with
EmbeddedRecipeHarvester that serves test recipes directly from embedded
assembly resources via IRecipeHarvester — no filesystem writes, no
shared lock, no ownership tracking. Any .recipe.json added to the
Fixtures/ folder is discovered automatically.

Remove all SetEnvironmentVariable calls from OrchardTestServer. Database
configuration is now injected per-host via builder.Configuration (which
takes priority over env vars), making each host self-contained with zero
process-global side effects.

Replace SqliteConnection.ClearAllPools() with a targeted ClearPool()
scoped to the fixture's specific database file.

These changes remove every shared-mutable-state blocker, enabling full
parallel execution of functional test fixtures.

* Update test/OrchardCore.Tests.Functional/README.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update test/OrchardCore.Tests.Functional/Helpers/TenantHelper.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update test/OrchardCore.Tests.Functional/Helpers/EmbeddedRecipeHarvester.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix CA1845: use span-based string.Concat in EmbeddedRecipeHarvester

Replace Substring concatenation with string.Concat and AsSpan to
satisfy the CA1845 analyzer rule.

* Add Docker Hub credentials to functional_all_db service containers

Authenticate PostgreSQL and MySQL service container image pulls with
DOCKERHUB_USERNAME/DOCKERHUB_PASSWORD secrets to avoid Docker Hub
anonymous rate limits (100 pulls/6h) on shared GitHub Actions runners.

* Fix parallel database collision by clearing env vars at process start

ShellSettingsManager re-adds EnvironmentVariablesConfigurationProvider
with highest priority, overriding the per-fixture connection strings set
via builder.Configuration. All fixtures ended up using the same database,
causing 'Table already exists' errors under parallel execution.

Replace static readonly capture with CaptureAndClear that reads the CI
env vars once at static initialization and immediately removes them.
Each fixture's database config is then applied exclusively through
tenants.json and builder.Configuration without env var interference.

* Fix SaaS tenant setup on external databases and add run-db-tests.sh

Fix two issues that caused SaaS functional tests to fail on PostgreSQL,
MySQL, and SQL Server:

1. CreateTenantAsync skipped setting the table prefix when
   DatabaseConfigurationPreset was true (external DB on parent), because
   the #DatabaseProvider dropdown was not rendered and the entire block
   was guarded by its presence. Move table prefix handling outside the
   dropdown check so it always runs when the field is visible.

2. EnsureDatabaseExists kept leftover tables from previous runs, causing
   "database, table prefix and schema are already in use" validation
   errors. Replace with EnsureCleanDatabase that drops and recreates
   the fixture database. Uses pg_terminate_backend for Postgres < 13
   compatibility.

Also add run-db-tests.sh helper script to run functional tests against
all database providers locally via Docker, and force English locale so
test assertions match on non-English machines. Update README with usage.

* cleanup

* remove maxParallelThreads

* Fix README to match current test infrastructure behavior

Describe CaptureAndClear pattern instead of claiming no env vars are
mutated, note that fixture databases are dropped and recreated for
clean state, and clarify that maxParallelThreads defaults to CPU core
count when omitted from xunit.runner.json.

* Add powershell script

* revert changes on SharedViewCompilerProvider

* Remove unecessary xunit.runner.json file

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Zoltán Lehóczky <zoltan.lehoczky@lombiq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants